Skip to content

Allow removing all instruction steps - #1599

Open
maxelkins wants to merge 6 commits into
mainfrom
remove-all-instruction-steps
Open

Allow removing all instruction steps#1599
maxelkins wants to merge 6 commits into
mainfrom
remove-all-instruction-steps

Conversation

@maxelkins

@maxelkins maxelkins commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Summary

Teachers can remove either the current instruction step or every step in the project. The modal defaults to the current step, so removing everything requires an explicit choice.

When the project has only one step, the scope options are hidden and the existing "Remove instructions" action remains.

Fixes https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1705

Implementation

  • Add Design System Fieldset and RadioInput controls to the removal modal.
  • Reset the scope to the current step whenever the modal opens.
  • Change the confirmation text to match the selected scope.
  • Add the all-steps removal path and cover both scopes with tests.
  • Upgrade Design System React to 2.10.4 for themeable radio dots and transparent fieldsets.
  • Stop broad modal input selectors from overriding Design System controls.
  • Allow GeneralModal callers to supply a class name.

The companion standalone change supplies the dark Design System tokens when the modal is portalled into the host application: https://github.com/RaspberryPiFoundation/editor-standalone/pull/1075

Checks

  • yarn lint
  • yarn stylelint
  • 1,080 unit tests
  • Production build

Screenshot

Screenshot.2026-08-14.at.17.18.05.mp4

@maxelkins
maxelkins temporarily deployed to previews/1599/merge August 14, 2026 16:00 — with GitHub Actions Inactive
@maxelkins

Copy link
Copy Markdown
Contributor Author

Bold radio text is incorrect (shows in standalone) another symptom of react portals moving outside shadow dom.

https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1716

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds deliberate scope selection when removing instruction steps.

Changes:

  • Supports removing the current step or all steps.
  • Updates modal copy, controls, styling, and translations.
  • Adds tests for scope selection and removal behavior.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/utils/instructionSteps.js Defines removal scope constants.
src/components/Modals/RemoveInstructionStepModal.jsx Adds scope radio controls.
src/components/Modals/GeneralModal.jsx Supports custom modal classes.
src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.jsx Implements scoped removal behavior.
src/components/Menus/Sidebar/InstructionsPanel/InstructionsPanel.test.jsx Tests removal scopes and defaults.
src/assets/stylesheets/Modal.scss Limits input styling to text inputs.
src/assets/stylesheets/Instructions.scss Styles the removal modal.
public/translations/en.json Adds updated removal copy.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@jamiebenstead jamiebenstead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@maxelkins
maxelkins marked this pull request as draft August 18, 2026 08:54
@maxelkins
maxelkins temporarily deployed to previews/1599/merge August 18, 2026 08:54 — with GitHub Actions Inactive
@maxelkins
maxelkins force-pushed the remove-all-instruction-steps branch from 538bf45 to 2be0d17 Compare August 18, 2026 08:58
@maxelkins
maxelkins temporarily deployed to previews/1599/merge August 18, 2026 08:58 — with GitHub Actions Inactive
@maxelkins
maxelkins force-pushed the remove-all-instruction-steps branch from 2be0d17 to 770fccb Compare August 20, 2026 10:49
@maxelkins
maxelkins temporarily deployed to previews/1599/merge August 20, 2026 10:49 — with GitHub Actions Inactive
@maxelkins
maxelkins force-pushed the remove-all-instruction-steps branch from 770fccb to 7987f52 Compare August 28, 2026 13:13
@maxelkins
maxelkins temporarily deployed to previews/1599/merge August 28, 2026 13:13 — with GitHub Actions Inactive
Upgrade Design System React to 2.10.4 so fieldsets no longer paint a light background and radio dots can follow the modal's input colour. Set the modal form tokens in dark mode rather than overriding the design system component styles.
@maxelkins
maxelkins temporarily deployed to previews/1599/merge August 28, 2026 13:19 — with GitHub Actions Inactive
@maxelkins
maxelkins marked this pull request as ready for review August 28, 2026 13:20
@maxelkins

maxelkins commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Design System React 2.10.4 now supports themeable checked radio colours and transparent fieldsets. I have upgraded the dependency, set the dark modal input tokens.

@jamiebenstead jamiebenstead left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still looks good to me :)

Move Design System form tokens from modal styling into the editor dark theme so all form controls share the same input, label, and focus colours.
@maxelkins maxelkins changed the title feat: add ability to delete all instruction steps Allow removing all instruction steps Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants